Skip to content

Conversation

@bignimbus
Copy link
Contributor

Pretty straightforward; most other linters I've worked with allow for a space between /* and the linter name for embedded ruleset comments. I think csslint should support the same as a very small enhancement. Thanks for your consideration!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not allow more than one space in the embedded ruleset? (i.e. use "*" instead of "?")

Also, it would be cool if you could add a test to tests/core/CSSLint.js to make sure the behaviour works the way you expect. 😉 Something like:

"Spaces should be allowed in embedded rulesets": function(){
    var result = CSSLint.verify("/* csslint bogus, adjoining-classes:true, box-sizing:false */\n.foo.bar{}", {
      "text-indent": 1,
      "box-sizing": 1
    });

    Assert.areEqual(2, result.ruleset["adjoining-classes"]);
    Assert.areEqual(1, result.ruleset["text-indent"]);
    Assert.areEqual(0, result.ruleset["box-sizing"]);
}

would work.

@bignimbus
Copy link
Contributor Author

Works for me. Will definitely add the test. Should we limit the amount of whitespace?

/*               csslint ... */

Would work without an enforced convention. Is that desired behavior?

@bwinton
Copy link

bwinton commented Jun 2, 2015

I don't think I'ld use it myself, but I can't think of a particularly good reason to restrict it…
(Maybe people want to line up their csslint statements?

background-color: rebeccapurple; /* csslint … */
border: 1px solid blue; /*          csslint … */

Yeah, they should probably line up the /*s in that case, too… So I think I'm happy with or without a limit.)

@bignimbus
Copy link
Contributor Author

Sounds good.

@bwinton
Copy link

bwinton commented Jun 2, 2015

Looks great! Thank you!

@XhmikosR XhmikosR added this to the 1.0.0 milestone Jan 9, 2016
XhmikosR added a commit that referenced this pull request Jan 9, 2016
optional space between /* and csslint in embedded ruleset; fixes #549
@XhmikosR XhmikosR merged commit fce3c82 into CSSLint:master Jan 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants